-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[IR] Add utilities for manipulating length of MemIntrinsic [nfc] #153856
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hoal is simply to reduce direct usage of getLength and setLength so that if we end up moving memset.pattern (whose length is in elements) there are fewer places to audit.
@llvm/pr-subscribers-llvm-ir Author: Philip Reames (preames) ChangesHoal is simply to reduce direct usage of getLength and setLength so that if we end up moving memset.pattern (whose length is in elements) there are fewer places to audit. Full diff: https://github.com/llvm/llvm-project/pull/153856.diff 6 Files Affected:
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 2e1389633d7e4..eb0440f500735 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -987,6 +987,13 @@ template <typename Derived> class MemIntrinsicBase : public IntrinsicInst {
const Use &getLengthUse() const { return getArgOperandUse(ARG_LENGTH); }
Use &getLengthUse() { return getArgOperandUse(ARG_LENGTH); }
+ std::optional<APInt> getLengthInBytes() const {
+ ConstantInt *C = dyn_cast<ConstantInt>(getLength());
+ if (!C)
+ return std::nullopt;
+ return C->getValue();
+ }
+
/// This is just like getRawDest, but it strips off any cast
/// instructions (including addrspacecast) that feed it, giving the
/// original input. The returned value is guaranteed to be a pointer.
@@ -1022,6 +1029,10 @@ template <typename Derived> class MemIntrinsicBase : public IntrinsicInst {
"setLength called with value of wrong type!");
setArgOperand(ARG_LENGTH, L);
}
+
+ void setLength(uint64_t L) {
+ setLength(ConstantInt::get(getLength()->getType(), L));
+ }
};
/// Common base class for all memory transfer intrinsics. Simply provides
diff --git a/llvm/lib/CodeGen/SafeStack.cpp b/llvm/lib/CodeGen/SafeStack.cpp
index 908ed96172615..6f373a53020d1 100644
--- a/llvm/lib/CodeGen/SafeStack.cpp
+++ b/llvm/lib/CodeGen/SafeStack.cpp
@@ -262,7 +262,7 @@ bool SafeStack::IsMemIntrinsicSafe(const MemIntrinsic *MI, const Use &U,
return true;
}
- const auto *Len = dyn_cast<ConstantInt>(MI->getLength());
+ auto Len = MI->getLengthInBytes();
// Non-constant size => unsafe. FIXME: try SCEV getRange.
if (!Len) return false;
return IsAccessSafe(U, Len->getZExtValue(), AllocaPtr, AllocaSize);
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 01da01239382b..ff4da3378f82d 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -2008,9 +2008,8 @@ struct AAPointerInfoCallSiteArgument final : AAPointerInfoFloating {
// destination) and second (=source) arguments as we know how they are
// accessed.
if (auto *MI = dyn_cast_or_null<MemIntrinsic>(getCtxI())) {
- ConstantInt *Length = dyn_cast<ConstantInt>(MI->getLength());
int64_t LengthVal = AA::RangeTy::Unknown;
- if (Length)
+ if (auto Length = MI->getLengthInBytes())
LengthVal = Length->getSExtValue();
unsigned ArgNo = getIRPosition().getCallSiteArgNo();
ChangeStatus Changed = ChangeStatus::UNCHANGED;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index d7a2ef722c846..095a76c2e3ad3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -133,7 +133,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
// wouldn't be constant), and this must be a noop.
if (!isModSet(AA->getModRefInfoMask(MI->getDest()))) {
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -141,7 +141,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
// (unless the transfer is volatile).
if (hasUndefSource(MI) && !MI->isVolatile()) {
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -211,7 +211,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
}
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MemOpLength->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -229,7 +229,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
// wouldn't be constant), and this must be a noop.
if (!isModSet(AA->getModRefInfoMask(MI->getDest()))) {
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -238,7 +238,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
// value. Change to PoisonValue once #52930 is resolved.
if (isa<UndefValue>(MI->getValue())) {
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -279,7 +279,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
S->setOrdering(AtomicOrdering::Unordered);
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(LenC->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -1753,9 +1753,9 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
// Intrinsics cannot occur in an invoke or a callbr, so handle them here
// instead of in visitCallBase.
if (auto *MI = dyn_cast<AnyMemIntrinsic>(II)) {
- if (ConstantInt *NumBytes = dyn_cast<ConstantInt>(MI->getLength())) {
+ if (auto NumBytes = MI->getLengthInBytes()) {
// memmove/cpy/set of zero bytes is a noop.
- if (NumBytes->isNullValue())
+ if (NumBytes->isZero())
return eraseInstFromFunction(CI);
// For atomic unordered mem intrinsics if len is not a positive or
diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index 8093e44245d20..aa6a3abb34ac9 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -685,15 +685,13 @@ static bool tryToShorten(Instruction *DeadI, int64_t &DeadStart,
<< "\n KILLER [" << ToRemoveStart << ", "
<< int64_t(ToRemoveStart + ToRemoveSize) << ")\n");
- Value *DeadWriteLength = DeadIntrinsic->getLength();
- Value *TrimmedLength = ConstantInt::get(DeadWriteLength->getType(), NewSize);
- DeadIntrinsic->setLength(TrimmedLength);
+ DeadIntrinsic->setLength(NewSize);
DeadIntrinsic->setDestAlignment(PrefAlign);
Value *OrigDest = DeadIntrinsic->getRawDest();
if (!IsOverwriteEnd) {
Value *Indices[1] = {
- ConstantInt::get(DeadWriteLength->getType(), ToRemoveSize)};
+ ConstantInt::get(DeadIntrinsic->getLength()->getType(), ToRemoveSize)};
Instruction *NewDestGEP = GetElementPtrInst::CreateInBounds(
Type::getInt8Ty(DeadIntrinsic->getContext()), OrigDest, Indices, "",
DeadI->getIterator());
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index d6e27aa20730b..b4e74bb1e58c0 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -3425,8 +3425,7 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
// Rewrite the size as needed.
if (NewEndOffset != EndOffset)
- II.setLength(ConstantInt::get(II.getLength()->getType(),
- NewEndOffset - NewBeginOffset));
+ II.setLength(NewEndOffset - NewBeginOffset);
return false;
}
// Record this instruction for deletion.
|
@llvm/pr-subscribers-llvm-transforms Author: Philip Reames (preames) ChangesHoal is simply to reduce direct usage of getLength and setLength so that if we end up moving memset.pattern (whose length is in elements) there are fewer places to audit. Full diff: https://github.com/llvm/llvm-project/pull/153856.diff 6 Files Affected:
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 2e1389633d7e4..eb0440f500735 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -987,6 +987,13 @@ template <typename Derived> class MemIntrinsicBase : public IntrinsicInst {
const Use &getLengthUse() const { return getArgOperandUse(ARG_LENGTH); }
Use &getLengthUse() { return getArgOperandUse(ARG_LENGTH); }
+ std::optional<APInt> getLengthInBytes() const {
+ ConstantInt *C = dyn_cast<ConstantInt>(getLength());
+ if (!C)
+ return std::nullopt;
+ return C->getValue();
+ }
+
/// This is just like getRawDest, but it strips off any cast
/// instructions (including addrspacecast) that feed it, giving the
/// original input. The returned value is guaranteed to be a pointer.
@@ -1022,6 +1029,10 @@ template <typename Derived> class MemIntrinsicBase : public IntrinsicInst {
"setLength called with value of wrong type!");
setArgOperand(ARG_LENGTH, L);
}
+
+ void setLength(uint64_t L) {
+ setLength(ConstantInt::get(getLength()->getType(), L));
+ }
};
/// Common base class for all memory transfer intrinsics. Simply provides
diff --git a/llvm/lib/CodeGen/SafeStack.cpp b/llvm/lib/CodeGen/SafeStack.cpp
index 908ed96172615..6f373a53020d1 100644
--- a/llvm/lib/CodeGen/SafeStack.cpp
+++ b/llvm/lib/CodeGen/SafeStack.cpp
@@ -262,7 +262,7 @@ bool SafeStack::IsMemIntrinsicSafe(const MemIntrinsic *MI, const Use &U,
return true;
}
- const auto *Len = dyn_cast<ConstantInt>(MI->getLength());
+ auto Len = MI->getLengthInBytes();
// Non-constant size => unsafe. FIXME: try SCEV getRange.
if (!Len) return false;
return IsAccessSafe(U, Len->getZExtValue(), AllocaPtr, AllocaSize);
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 01da01239382b..ff4da3378f82d 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -2008,9 +2008,8 @@ struct AAPointerInfoCallSiteArgument final : AAPointerInfoFloating {
// destination) and second (=source) arguments as we know how they are
// accessed.
if (auto *MI = dyn_cast_or_null<MemIntrinsic>(getCtxI())) {
- ConstantInt *Length = dyn_cast<ConstantInt>(MI->getLength());
int64_t LengthVal = AA::RangeTy::Unknown;
- if (Length)
+ if (auto Length = MI->getLengthInBytes())
LengthVal = Length->getSExtValue();
unsigned ArgNo = getIRPosition().getCallSiteArgNo();
ChangeStatus Changed = ChangeStatus::UNCHANGED;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index d7a2ef722c846..095a76c2e3ad3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -133,7 +133,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
// wouldn't be constant), and this must be a noop.
if (!isModSet(AA->getModRefInfoMask(MI->getDest()))) {
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -141,7 +141,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
// (unless the transfer is volatile).
if (hasUndefSource(MI) && !MI->isVolatile()) {
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -211,7 +211,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
}
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MemOpLength->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -229,7 +229,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
// wouldn't be constant), and this must be a noop.
if (!isModSet(AA->getModRefInfoMask(MI->getDest()))) {
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -238,7 +238,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
// value. Change to PoisonValue once #52930 is resolved.
if (isa<UndefValue>(MI->getValue())) {
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -279,7 +279,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
S->setOrdering(AtomicOrdering::Unordered);
// Set the size of the copy to 0, it will be deleted on the next iteration.
- MI->setLength(Constant::getNullValue(LenC->getType()));
+ MI->setLength((uint64_t)0);
return MI;
}
@@ -1753,9 +1753,9 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
// Intrinsics cannot occur in an invoke or a callbr, so handle them here
// instead of in visitCallBase.
if (auto *MI = dyn_cast<AnyMemIntrinsic>(II)) {
- if (ConstantInt *NumBytes = dyn_cast<ConstantInt>(MI->getLength())) {
+ if (auto NumBytes = MI->getLengthInBytes()) {
// memmove/cpy/set of zero bytes is a noop.
- if (NumBytes->isNullValue())
+ if (NumBytes->isZero())
return eraseInstFromFunction(CI);
// For atomic unordered mem intrinsics if len is not a positive or
diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index 8093e44245d20..aa6a3abb34ac9 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -685,15 +685,13 @@ static bool tryToShorten(Instruction *DeadI, int64_t &DeadStart,
<< "\n KILLER [" << ToRemoveStart << ", "
<< int64_t(ToRemoveStart + ToRemoveSize) << ")\n");
- Value *DeadWriteLength = DeadIntrinsic->getLength();
- Value *TrimmedLength = ConstantInt::get(DeadWriteLength->getType(), NewSize);
- DeadIntrinsic->setLength(TrimmedLength);
+ DeadIntrinsic->setLength(NewSize);
DeadIntrinsic->setDestAlignment(PrefAlign);
Value *OrigDest = DeadIntrinsic->getRawDest();
if (!IsOverwriteEnd) {
Value *Indices[1] = {
- ConstantInt::get(DeadWriteLength->getType(), ToRemoveSize)};
+ ConstantInt::get(DeadIntrinsic->getLength()->getType(), ToRemoveSize)};
Instruction *NewDestGEP = GetElementPtrInst::CreateInBounds(
Type::getInt8Ty(DeadIntrinsic->getContext()), OrigDest, Indices, "",
DeadI->getIterator());
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index d6e27aa20730b..b4e74bb1e58c0 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -3425,8 +3425,7 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
// Rewrite the size as needed.
if (NewEndOffset != EndOffset)
- II.setLength(ConstantInt::get(II.getLength()->getType(),
- NewEndOffset - NewBeginOffset));
+ II.setLength(NewEndOffset - NewBeginOffset);
return false;
}
// Record this instruction for deletion.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
Goal is simply to reduce direct usage of getLength and setLength so that if we end up moving memset.pattern (whose length is in elements) there are fewer places to audit.